fix(serverless): redact env values in debug response logs - #92
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. To trigger a review, include ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR centralizes --debug response logging for the serverless control-plane client behind a shared helper that redacts JSON "value" fields on successful (2xx/3xx) responses, preventing plaintext env var / secret values from being persisted in debug logs.
Changes:
- Replaces per-callsite debug logging in serverless API methods with a shared
Client.logResponse(...)helper. - Adds JSON redaction logic that recursively replaces
"value"fields with"[redacted]"for success responses while leaving error bodies unchanged. - Introduces unit tests covering redaction behavior and success-vs-error logging behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/api/serverless/secrets.go | Routes secrets endpoints through the shared response logger (now includes body logging). |
| internal/api/serverless/env.go | Routes env var endpoints through the shared response logger so "value" fields are redacted on success. |
| internal/api/serverless/client.go | Routes additional control-plane endpoints through the shared response logger for consistent debug output. |
| internal/api/serverless/log.go | Implements centralized debug response logging and recursive JSON "value" redaction for success responses. |
| internal/api/serverless/log_test.go | Adds tests validating redaction behavior and the success/error logging split. |
Suppressed comments (3)
internal/api/serverless/secrets.go:83
- This now logs the full /v1/secrets response body under --debug. Although secret values are not returned, the response can include user-provided
metadata(see gen.Secret.Metadata) which may contain sensitive data and is not redacted byredactJSONValues(it only redacts the key "value"). Consider keeping the previous behavior (log status/path only) for secrets endpoints by not passing the response body here.
c.logResponse(ctx, "/v1/secrets", resp.StatusCode(), resp.Body)
internal/api/serverless/secrets.go:152
- This now logs the full /v1/secrets/{name} response body under --debug. Secret responses can include user-provided
metadatawhich is not redacted byredactJSONValues(it only redacts the key "value"), so sensitive metadata could end up in logs. Consider not logging bodies for secrets endpoints.
c.logResponse(ctx, "/v1/secrets/"+name, resp.StatusCode(), resp.Body)
internal/api/serverless/secrets.go:218
- This logs the /v1/deployments/{id}/secrets response body under --debug. While values are not returned, attached secret records may still include user-provided metadata via the underlying Secret, which isn't redacted by
redactJSONValues. If the goal is specifically to avoid persisting secret-related data in logs, consider omitting bodies for secrets endpoints.
c.logResponse(ctx, "/v1/deployments/"+deploymentID+"/secrets", resp.StatusCode(), resp.Body)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Plaintext environment-variable values were written to --debug logs via response bodies. Route control-plane debug logging through a helper that redacts JSON "value" fields on 2xx/3xx responses. Co-authored-by: Cursor <cursoragent@cursor.com>
Secret responses can include opaque user metadata that is not covered by value redaction. Log path and status only for secrets endpoints. Co-authored-by: Cursor <cursoragent@cursor.com>
1726b44 to
7ec365d
Compare
Summary
--debuglogging through a shared helper that redacts JSON"value"fields on 2xx/3xx response bodies so plaintext env vars are not persisted in logs.feat/runserv-504soUpdateDeployment(apps scale) is covered as well.Test plan
go test -race ./internal/api/serverless/--debugonrunware serverless apps env list/env setshows[redacted]for values--debugMade with Cursor